gh-148390: fix undefined behavior of memoryview(...).cast("?")#148454
gh-148390: fix undefined behavior of memoryview(...).cast("?")#148454picnixz merged 8 commits intopython:mainfrom
memoryview(...).cast("?")#148454Conversation
picnixz
requested review from
encukou,
serhiy-storchaka,
skirpichev and
vstinner
StanFromIreland
left a comment
StanFromIreland
left a comment
There was a problem hiding this comment.
If the UB is fixed, please remove it from the suppressions list:
cpython/Tools/ubsan/suppressions.txt
Lines 12 to 13 in 3ab94d6
skirpichev
left a comment
skirpichev
left a comment
There was a problem hiding this comment.
Looks good.
Though, maybe move macro closer to unpack_single() definition?
The problem is that it's being used in two places so... I preferred moving it where we had "helpers". |
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
I confirm that without the fix, the test fails and logs an UBSan error:
test_compare_equal (test.test_memoryview.ArrayMemoryviewTest.test_compare_equal) ...
Objects/memoryobject.c:3032:15: runtime error: load of value 2, which is not a valid value for type '_Bool'
Objects/memoryobject.c:3032:15: runtime error: load of value 2, which is not a valid value for type '_Bool'
Objects/memoryobject.c:1814:15: runtime error: load of value 2, which is not a valid value for type '_Bool'
FAIL
======================================================================
FAIL: test_compare_equal (test.test_memoryview.ArrayMemoryviewTest.test_compare_equal)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/vstinner/python/main/Lib/test/test_memoryview.py", line 628, in test_compare_equal
self.assertEqual(m1a.tolist(), [False, True, False])
~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: Lists differ: [False, False, False] != [False, True, False]
First differing element 1:
False
True
- [False, False, False]
? ^^^^
+ [False, True, False]
? ^^^
I tested Python built with:
./configure --with-pydebug --with-undefined-behavior-sanitizer && make clean
Misc/NEWS.d/next/Core_and_Builtins/2026-04-12-17-27-28.gh-issue-148390.MAhw7F.rst
Outdated
Show resolved
Hide resolved
|
@serhiy-storchaka I addressed the comments, so you can look at the last 3 commits if you want to see the update |
|
Btw, what's the policy with |
|
I think it is only issue if its value can conflict between subinterpreters, or if references to it can outlive the dynamically loaded extension., or (in C++) when the constructor is executed at runtime and race conditions are possible. This is not a such case. |
Totally fine for immutable data like |
Co-authored-by: Petr Viktorin <encukou@gmail.com>
picnixz
added
needs backport to 3.13
|
Thanks @picnixz for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
|
Thanks @picnixz for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13. |
|
Sorry, @picnixz, I could not cleanly backport this to |
|
Sorry, @picnixz, I could not cleanly backport this to |
|
Huh. Ok. Well.. I am not really available to make the backports and will be leaving tomorrow for a week sooo.... if someone wants to do the backports they can (otherwise I'll do it when I'm back) |
7 participants
memoryview.cast("?")has undefined behavior on some platforms/compilers #148390